Skip to content

feat(nmrs): OpenVPN compression and proxy support#315

Draft
stoutes wants to merge 5 commits intocachebag:dev-openvpnfrom
stoutes:openvpn-compression-and-proxy-support
Draft

feat(nmrs): OpenVPN compression and proxy support#315
stoutes wants to merge 5 commits intocachebag:dev-openvpnfrom
stoutes:openvpn-compression-and-proxy-support

Conversation

@stoutes
Copy link
Copy Markdown
Collaborator

@stoutes stoutes commented Apr 1, 2026

Summary

Part of #288, closes #292, closes #299.

Adds OpenVPN compression and proxy support to the builder layer, including
the build_openvpn_connection() function and all supporting types.

Changes

api/models/vpn.rs

  • Added OpenVpnCompression enum mapping to NM compress and comp-lzo keys
    • Lzo marked #[deprecated]comp-lzo is deprecated upstream in favour of compress
    • All compression variants except No carry VORACLE vulnerability warnings in their doc comments
  • Added OpenVpnProxy enum with Http and Socks variants mapping to NM proxy keys
  • Added OpenVpnConfig struct with builder methods for port, compression, proxy, DNS, and UUID
  • All three types exported via pub use vpn::* in api/models/mod.rs and re-exported at crate root in lib.rs

api/builders/vpn.rs

  • Added build_openvpn_connection() — builds a complete NM settings dict for OpenVPN
    • connection.type = "vpn", vpn.service-type = "org.freedesktop.NetworkManager.openvpn"
    • All config serialized into flat vpn.data as Vec<(String, String)>
    • Compression dispatches on OpenVpnCompression variants to correct NM keys
    • Proxy dispatches on OpenVpnProxy variants, validates port is non-zero
    • #[allow(deprecated)] on the Lzo arm inside the builder so the function itself doesn't warn
  • Added 14 unit tests covering:
    • Basic connection structure (type, service-type, sections present)
    • All 5 compression modes
    • HTTP proxy with and without credentials
    • SOCKS proxy
    • Zero port rejection for both proxy types
    • Port, DNS, and empty remote validation

Notes

  • build_openvpn_connection is wired into core/vpn.rs dispatch but the OpenVpn arm in
    connect_vpn currently returns VpnFailed("not yet implemented") until the full
    connect flow from openvpn support #288 lands.

@cachebag
Copy link
Copy Markdown
Owner

cachebag commented Apr 1, 2026

@stoutes
Will TAL tomorrow. Make sure you rebase off of dev-ovpn, I just merged in our .ovpn recursive descent parser in #314 as well as #309.

I'd presume the builder should/would mesh with that in some way. We'd want to reconcile the approach we made in #309 for OpenVpnConfig as well with the version you created.

Please also update your PR description as well to mention the issues you tackled directly. It looks like you're doing both #292 and #299. In the future, I'd prefer you keep a PR to one issue, unless they're related in some fashion. (#288 is just the tracking issue for OpenVPN support so I wouldn't call them related).

Lastly, HUGE nitpick, but your commit messages should be feat(#<ISSUE_NUMBER>) or feat(nmrs). We don't use lib or the verbose feature.

@cachebag cachebag changed the base branch from master to dev-openvpn April 1, 2026 12:26
@cachebag cachebag force-pushed the dev-openvpn branch 2 times, most recently from e52daf0 to 6f56cf5 Compare April 1, 2026 19:03
@cachebag cachebag added nmrs Changes to nmrs refactor Change or improve code vpn Changes to VPN surface labels Apr 1, 2026
@cachebag cachebag changed the title Openvpn compression and proxy support feat(nmrs): OpenVPN compression and proxy support Apr 1, 2026
@cachebag cachebag force-pushed the dev-openvpn branch 2 times, most recently from c1acdfe to 5d65aa0 Compare April 1, 2026 19:11
@stoutes
Copy link
Copy Markdown
Collaborator Author

stoutes commented Apr 1, 2026

I'd presume the builder should/would mesh with that in some way. We'd want to reconcile the approach we made in #309 for OpenVpnConfig as well with the version you created.

So for the meshing are you thinking of some sort of conversion so the parser output can flow into the builder?

@cachebag
Copy link
Copy Markdown
Owner

cachebag commented Apr 2, 2026

I'd presume the builder should/would mesh with that in some way. We'd want to reconcile the approach we made in #309 for OpenVpnConfig as well with the version you created.

So for the meshing are you thinking of some sort of conversion so the parser output can flow into the builder?

@stoutes
No, sorry I should have been clearer. What I meant is just that #309 was already merged into dev-openvpn. The struct that was developed in there looks like this

pub struct OpenVpnConfig {
    pub name: String,
    pub remote: String,
    pub port: u16,
    pub tcp: bool,
    pub auth_type: Option<OpenVpnAuthType>,
    pub auth: Option<String>,
    pub cipher: Option<String>,
    pub dns: Option<Vec<String>>,
    pub mtu: Option<u32>,
    pub uuid: Option<Uuid>,
    pub ca_cert: Option<String>,
    pub client_cert: Option<String>,
    pub client_key: Option<String>,
    pub key_password: Option<String>,
    pub username: Option<String>,
    pub password: Option<String>,
}

Slightly different from yours.

All you need to do is rebase onto dev-ovpn to get the latest changes. And then extend the existing OpenVpnConfig from #309 with your new compression and proxy fields instead of creating a second struct. Keep all the auth/cipher/tcp/mtu fields since the builder needs to handle more than just TLS. Then write a TryFrom<OvpnFile> for OpenVpnConfig so the parser output flows straight into the builder, and update build_openvpn_connection to dispatch on auth_type instead of hardcoding connection-type: tls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nmrs Changes to nmrs refactor Change or improve code vpn Changes to VPN surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants